Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Score documents: Document selection and selection output #710

Merged
merged 1 commit into from
Sep 6, 2021
Merged

Score documents: Document selection and selection output #710

merged 1 commit into from
Sep 6, 2021

Conversation

PrimozGodec
Copy link
Collaborator

Issue

Score documents widget is missing selection

Description of changes

Implemented selection in the table view and the Selected documents output

Includes
  • Code changes
  • Tests
  • Documentation

@PrimozGodec PrimozGodec force-pushed the score-documents-outputs branch 2 times, most recently from 29e9585 to 8186e3a Compare August 18, 2021 07:11
@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2021 •
edited
Loading

Codecov Report

Merging #710 (f38504a) into master (7a5f5fe) will increase coverage by 0.11%.
The diff coverage is 92.78%.

❗ Current head f38504a differs from pull request most recent head 9d86427. Consider uploading reports for the commit 9d86427 to get more accurate results

@@            Coverage Diff             @@
##           master     #710      +/-   ##
==========================================
+ Coverage   74.09%   74.21%   +0.11%     
==========================================
  Files          72       72              
  Lines        9489     9566      +77     
  Branches     1294     1304      +10     
==========================================
+ Hits         7031     7099      +68     
- Misses       2212     2220       +8     
- Partials      246      247       +1

VesnaT
VesnaT reviewed Aug 20, 2021
View reviewed changes
Copy link
Contributor

@VesnaT VesnaT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The selection use usually considered as a schema_only setting.
  2. Have you considered adding a selection box. Something like:
    image

orangecontrib/text/widgets/owscoredocuments.py Outdated
@@ -4,6 +4,7 @@
from typing import List, Callable, Tuple, Union

import numpy as np
from Orange.widgets.utils.annotated_data import create_annotated_table
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nicer, if the imports were more structured.

@PrimozGodec
Copy link
Collaborator Author

The selection use usually considered as a schema_only setting.

Totally forgot that it would be nice if the selection is remembered. Will implement it

Have you considered adding a selection box. Something like:

I do not like selection boxes too much, since users can easily do selection in the table itself, but if other widgets have them I can also implement it here.

It would be nicer, if the imports were more structured.

I forgot to run isort on this one. Will fix it.

@PrimozGodec PrimozGodec marked this pull request as draft August 20, 2021 14:11
@PrimozGodec PrimozGodec force-pushed the score-documents-outputs branch 2 times, most recently from 2ea96df to f08bfaa Compare August 23, 2021 10:24
@PrimozGodec PrimozGodec marked this pull request as ready for review August 23, 2021 10:25
@PrimozGodec PrimozGodec force-pushed the score-documents-outputs branch 3 times, most recently from d4a8fd4 to 77d35b6 Compare August 25, 2021 12:29
@PrimozGodec PrimozGodec requested a review from VesnaT August 25, 2021 12:49
@PrimozGodec PrimozGodec force-pushed the score-documents-outputs branch 3 times, most recently from 1407787 to f38504a Compare August 31, 2021 08:55
VesnaT
VesnaT requested changes Sep 6, 2021
View reviewed changes
orangecontrib/text/widgets/owscoredocuments.py Outdated
@@ -183,12 +210,20 @@ def callback(i: float) -> None:
state.set_partial_result((sm, aggregation, scs))


class SelectionMethods:
NONE, ALL, MANUAL, N_BEST = range(4)
ITEMS = "None", "All", "Manual", "Top words"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering the widget scores the documents, Top words should be renamed to Top documents.

orangecontrib/text/widgets/owscoredocuments.py Outdated
@@ -324,6 +369,31 @@ def _setup_control_area(self) -> None:
gui.rubber(self.controlArea)
gui.auto_send(self.buttonsArea, self, "auto_commit")

# select words box
box = gui.vBox(self.controlArea, "Select Words")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Select Words -> Select Documents
self.controlArea -> self.buttonsArea

@PrimozGodec PrimozGodec force-pushed the score-documents-outputs branch 2 times, most recently from 9d86427 to d53604b Compare September 6, 2021 09:29
@PrimozGodec PrimozGodec force-pushed the score-documents-outputs branch from d53604b to b521d15 Compare September 6, 2021 09:32
@VesnaT VesnaT merged commit 0450823 into biolab:master Sep 6, 2021
@PrimozGodec PrimozGodec deleted the score-documents-outputs branch September 6, 2021 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VesnaT VesnaT VesnaT requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PrimozGodec @codecov-commenter @VesnaT